feat: SSH certificate authentication support#327
Conversation
Adds support for SSH certificate-based authentication with automatic fallback to key-based auth when certificates are invalid or expired. - Auto-detect companion certificate files (e.g., id_ed25519-cert.pub) - Validate certificate timing (expiry, not-yet-valid) - Emit warning events for certificate issues while allowing connection - Show toast notifications for certificate problems - Document known limitation: agent-held certificates not supported 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add certificate config UI (path/paste modes) to SSH settings modal - Improve certificate validation with expiry/not-yet-valid warnings - Add integration tests for certificate authentication - Update Docker test infrastructure with CA key generation - Fix toast overflow for long messages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace 50-line manual calendar math with OffsetDateTime::from().to_string() from the time crate (already a dependency). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
De-duplicate the 3-line publickey auth pattern that was repeated in each certificate validation fallback path. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace nested match statements with if-let early returns for clearer control flow in key_auth_from_content_with_cert and key_auth_with_cert_config. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Some WorkspaceError variants (WorkspaceNotWatched, RunbookNotFound, etc.) don't have a message field. Use type guard to safely access it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
42b5442 to
17d96d9
Compare
|
@greptile plz |
Greptile OverviewGreptile SummaryThis PR adds SSH certificate authentication support with per-block configuration. Users can specify identity keys and certificates via three modes: default (system keys), path-based, or paste-content. The implementation includes: Core Features:
Architecture:
Key Issue Found: Important Files ChangedFile Analysis
|
| result = pool_guard.connect_with_config(&host, Some(username.as_str()), None, Some(connect_cancel_rx), ssh_config.as_ref()) => { | ||
| tracing::trace!("SSH connection to {host} with username {username} successful"); | ||
| result.map_err(SshPoolConnectionError::from) | ||
| result.map(|(session, _auth_result)| session).map_err(SshPoolConnectionError::from) |
There was a problem hiding this comment.
The auth_result returned from connect_with_config contains warnings about certificate issues, but in the Exec message handler, these warnings are discarded (.map(|(session, _auth_result)| session)).
This means SSH certificate warnings (expired cert, load failed, etc.) are NOT emitted to the user when executing commands via SSH - they're only emitted when opening PTY sessions (see line 815 in OpenPty handler).
This is an inconsistency: users running commands over SSH won't see certificate warnings, but users opening interactive shells will. Consider emitting warnings here as well, similar to how it's done in the OpenPty handler (lines 815+ and terminal.rs lines 309-351).
There was a problem hiding this comment.
Claude finished @ellie's task in 6m 13s —— View job
Fixed SSH certificate warning inconsistency ✓
Todo:
- Read ssh_pool.rs to understand current Exec handler implementation
- Read OpenPty handler to understand warning emission pattern
- Read terminal.rs to understand the full warning flow
- Design solution approach
- Implement warning emission in Exec handler
- Update script.rs to handle and emit warnings
- Verify the code compiles
- Commit and push changes
Summary:
The issue was that SSH certificate warnings were being emitted for PTY sessions but NOT for command execution, creating an inconsistency where users running commands wouldn't see certificate warnings.
Changes made:
- Added
warnings_txoptional channel parameter to theExecmessage in ssh_pool.rs - Modified the Exec handler (line 618) to capture
auth_result.warningsinstead of discarding them - Warnings are sent through the new channel to the caller after SSH connection
- Updated script.rs to receive and emit these warnings as GC events (matching the terminal.rs pattern at lines 309-351)
This ensures SSH certificate warnings (expired, not-yet-valid, load failed) are now consistently emitted for both PTY sessions AND command execution flows.
Commit: 3b5ad0c
Certificate authentication warnings were being emitted for PTY sessions but not for command execution (exec). This caused an inconsistency where users running commands wouldn't see certificate warnings, but interactive shell users would. Changes: - Added warnings_tx channel to Exec message in ssh_pool.rs - Modified Exec handler to capture and send auth_result.warnings - Updated script.rs to receive and emit warnings as GC events - Warnings now flow consistently for both PTY and exec operations Fixes issue identified in code review where auth_result was discarded on line 618 during exec connection. Co-authored-by: Ellie Huxtable <ellie@users.noreply.github.com>
3b5ad0c to
f3ba5aa
Compare
Change Summary
Add SSH certificate authentication support with per-block certificate configuration. Users can now specify identity keys and certificates directly in SSH connect blocks, with local-only storage for per-user credentials on shared runbooks.
Motivation and details
This feature enables more flexible SSH authentication scenarios:
Tasks
ts(export)structs have changed)docs/(if any application behavior has changed)